Skip to content

fix(templatize): adapt IstioUpgrade step for ARO-Tools Value aksCluster - #6201

Merged
openshift-merge-bot[bot] merged 3 commits into
Azure:mainfrom
trevorwilliams2025:bump/aro-tools-istio-value
Jul 22, 2026
Merged

fix(templatize): adapt IstioUpgrade step for ARO-Tools Value aksCluster#6201
openshift-merge-bot[bot] merged 3 commits into
Azure:mainfrom
trevorwilliams2025:bump/aro-tools-istio-value

Conversation

@trevorwilliams2025

@trevorwilliams2025 trevorwilliams2025 commented Jul 22, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

What

  • Bump github.com/Azure/ARO-Tools/pipelines to current main (ca2cea2, includes Azure/ARO-Tools#265)
  • Adapt IstioUpgrade pipeline runner to resolve aksCluster as a Value via resolveValue (with ExecutionState outputs + id.ServiceGroup, and reject empty cluster name)
  • Remove use of removed dryRun field on IstioUpgradeStep
  • Propagate the same pipelines bump through workspace modules that depend on it (test, tooling/helmtest, tooling/pipeline-documentation) via make tidy

Why

ARO-Tools#265 changed IstioUpgradeStep.AKSCluster from string to Value and dropped dryRun. Without this consumer adapt, ARO-HCP templatize does not compile against current ARO-Tools, which blocks dependency bumps (e.g. SafeFly in #6173).

Testing

  • Unit tests: go test ./pkg/pipeline/... in tooling/templatize (passes locally). No new cases — compile/API adapt only.
  • Integration tests: not added; no runtime behavior change beyond resolving configRef/input through Value
  • E2E tests: not added; same justification

Special notes for your reviewer

  • Functional change is in tooling/templatize/pkg/pipeline/{istio.go,run.go}
  • go.mod/go.sum updates also include test, tooling/helmtest, and tooling/pipeline-documentation — required so make tidy / verify-deepcopy stays clean after the workspace pipelines bump (not optional extras)
  • Unblocks Mustafa’s ARO-Tools SafeFly bump once merged

PR Checklist

  • PR is scoped to a single task (no mixed concerns)
  • Title follows Conventional Commits format
  • Summary explains the "Why" behind the change
  • Linked to relevant ticket/issue
  • Screenshots included (if graph/UI/metrics changes)
  • Self-reviewed the diff
  • CI/CD checks are passing (ignore Tide)
  • Draft PR used for WIP (if applicable)
  • Commit history is clean (rebased/squashed)
  • Tricky code blocks are commented
  • Specific reviewers tagged
  • All comment threads resolved before merge
    If E2E tests are included:
  • E2E tests follow Principles of Good E2E Test Case Design
  • If new E2E use case is covered (via a new test or new check/verifier),
    demonstrate that the test is able to detect a defect/error and fail with
    proper error message and logs which communicates nature of the problem.

Copilot AI review requested due to automatic review settings July 22, 2026 00:05
@openshift-ci
openshift-ci Bot requested review from geoberle and janboll July 22, 2026 00:05
Copilot AI reviewed Jul 22, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates tooling/templatize to compile against the latest github.com/Azure/ARO-Tools/pipelines by adapting the IstioUpgrade runner to the updated IstioUpgradeStep schema (AKSCluster now a Value, dryRun removed). This unblocks downstream dependency bumps that require the newer ARO-Tools pipelines commit.

Changes:

  • Bump github.com/Azure/ARO-Tools/pipelines dependency to ca2cea2cb707.
  • Resolve IstioUpgradeStep.AKSCluster via resolveValue and thread the resolved cluster name into kubeconfig creation and Istio upgrade options.
  • Remove assignment to the removed DryRun field.

Reviewed changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.

File Description
tooling/templatize/pkg/pipeline/istio.go Updates IstioUpgrade runner to resolve aksCluster via resolveValue and removes DryRun usage.
tooling/templatize/go.mod Bumps the github.com/Azure/ARO-Tools/pipelines module version.
tooling/templatize/go.sum Updates sums for the new pipelines pseudo-version.

Comment thread tooling/templatize/pkg/pipeline/istio.go Outdated
Comment thread tooling/templatize/pkg/pipeline/istio.go Outdated
Copilot AI review requested due to automatic review settings July 22, 2026 00:38
@trevorwilliams2025
trevorwilliams2025 force-pushed the bump/aro-tools-istio-value branch from d9e8f29 to ba518ff Compare July 22, 2026 00:38
@trevorwilliams2025
trevorwilliams2025 force-pushed the bump/aro-tools-istio-value branch from ba518ff to 4f1c426 Compare July 22, 2026 00:40
Copilot AI reviewed Jul 22, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 9 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

tooling/templatize/pkg/pipeline/istio.go:43

  • resolveValue uses the passed serviceGroup when resolving input-based Values. Passing an empty serviceGroup here makes any future/use-case errors harder to diagnose (they’ll report missing service group ""). Passing id.ServiceGroup is a no-risk improvement and keeps this consistent with other resolveValue call sites.
	clusterName, err := resolveValue(step.AKSCluster, options.Configuration, nil, "")

Copilot AI review requested due to automatic review settings July 22, 2026 00:42
Copilot AI reviewed Jul 22, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 9 changed files in this pull request and generated no new comments.

@trevorwilliams2025
fix(templatize): complete IstioUpgrade aksCluster Value resolution
2cc4cb2 
Wire ExecutionState outputs and id.ServiceGroup into resolveValue, and
reject an empty resolved cluster name.
Copilot AI review requested due to automatic review settings July 22, 2026 00:47
Copilot AI reviewed Jul 22, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 10 changed files in this pull request and generated 1 comment.

Comment thread tooling/pipeline-documentation/go.mod
@trevorwilliams2025
trevorwilliams2025 marked this pull request as draft July 22, 2026 00:54
@trevorwilliams2025
trevorwilliams2025 marked this pull request as ready for review July 22, 2026 00:56
Copilot AI review requested due to automatic review settings July 22, 2026 00:59
Copilot AI reviewed Jul 22, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 6 out of 10 changed files in this pull request and generated no new comments.

@stevekuznetsov

stevekuznetsov commented Jul 22, 2026

Copy link
Copy Markdown
Contributor

/lgtm
/approve

@openshift-ci openshift-ci Bot added the lgtm label Jul 22, 2026
@openshift-ci

openshift-ci Bot commented Jul 22, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: stevekuznetsov, trevorwilliams2025

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot
openshift-merge-bot Bot merged commit fa93863 into Azure:main Jul 22, 2026
16 checks passed
@Mustafa-Ali-code Mustafa-Ali-code mentioned this pull request Jul 22, 2026
Open
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot Copilot left review comments
@geoberle geoberle Awaiting requested review from geoberle
@janboll janboll Awaiting requested review from janboll

Assignees

@stevekuznetsov stevekuznetsov

Labels

approved lgtm

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@trevorwilliams2025 @stevekuznetsov